-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
buffer: use FastBuffer when fill
is set to 0
#21989
Conversation
A large number of libraries seem to use Buffer.alloc(size, 0) instead of just Buffer.alloc(size). We don't need to follow the "create unsafe buffer and fill it" path (i.e. actually allocate and perform fill) in that situation, that is better handled by Uint8Array constructor. Buffer.alloc(size) and Buffer.alloc(size, 0) are equivalent, so use the same code path. Not performing the zero-fill manually and having the underlying memory allocator do it for us can improve speed and reduce the memory usage for situations where Buffer.alloc(size, 0) is used.
@ChALkeR - can you please show me where the zero-filling happens with the proposed changes? |
@gireeshpunathil It calls
This change makes |
Landed in b07852d |
A large number of libraries seem to use Buffer.alloc(size, 0) instead of just Buffer.alloc(size). We don't need to follow the "create unsafe buffer and fill it" path (i.e. actually allocate and perform fill) in that situation, that is better handled by Uint8Array constructor. Buffer.alloc(size) and Buffer.alloc(size, 0) are equivalent, so use the same code path. Not performing the zero-fill manually and having the underlying memory allocator do it for us can improve speed and reduce the memory usage for situations where Buffer.alloc(size, 0) is used. PR-URL: #21989 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
A large number of libraries seem to use Buffer.alloc(size, 0) instead of just Buffer.alloc(size). We don't need to follow the "create unsafe buffer and fill it" path (i.e. actually allocate and perform fill) in that situation, that is better handled by Uint8Array constructor. Buffer.alloc(size) and Buffer.alloc(size, 0) are equivalent, so use the same code path. Not performing the zero-fill manually and having the underlying memory allocator do it for us can improve speed and reduce the memory usage for situations where Buffer.alloc(size, 0) is used. PR-URL: #21989 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
A large number of libraries seem to use
Buffer.alloc(size, 0)
instead of justBuffer.alloc(size)
.We don't need to follow the «create unsafe buffer and fill it» path (i.e. actually allocate and perform fill) in that situation, that is better handled by
Uint8Array
constructor.Buffer.alloc(size)
andBuffer.alloc(size, 0)
are equivalent, so use the same code path.Not performing the zero-fill manually and having the underlying memory allocator do it for us can improve speed and reduce the memory usage for situations where
Buffer.alloc(size, 0)
is used.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesTests/benchmarks are not included as whether or not there would be improvement depends on the underlying memory allocator behavior (and probably also operating system memory management).
This is the difference on the current Node.js version (v10.7.0) and Linux 4.17.9:
As it can be seen,
Buffer.alloc(size)
does not actually consume and/or zero-fill physical memory on the time of construction. Underlying memory allocation mechanism is able to track that the memory is supposed to be zero-filled (ascalloc
behaves), and zero-filled pages could be returned on first read.That also affects speed in cases when memory is not read but is just overwritten:
This change makes
Buffer.alloc(size, 0)
follow the same code path as more performantBuffer.alloc(size)
, which should increase speed in some cases where the buffer is overwritten and reduce memory usage in some cases where part of the buffer is later thrown away without being used (i.e. where more memory than needed is allocated temporarily).I have seen usage of
Buffer.alloc(size, 0)
in many packages, including, but not limited to mysql2, hdkey, secp256k1 and more./cc @addaleax @bnoordhuis